Skip to content

Enforce focus on dropdown trigger #41711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Enforce focus on dropdown trigger #41711

wants to merge 6 commits into from
+98 −0

Conversation

@alpadev
Copy link
Contributor

@alpadev alpadev commented Aug 31, 2025 •
edited
Loading

Description

Enforce focus on the dropdown trigger after an item has been selected (and the dropdown is hidden).

Motivation & Context

See #35793

Regarding the implementation - I've seen some proposed implementation by @julien-deramond in another issue, but he mentioned it might be quite complex. This way the focus is only moved to the trigger, if the active focus resides within the dropdown menu. As such it shouldn't conflict if the focus was moved prior/in the event of being hidden.

I set the focus before the hidden event is dispatched, in case some implementor wanted to change the focus to something else, via the event handler (or in case the dropdown is being disposed in the handler - to prevent an error in case the element doesn't exist anymore). Although in that case, I could imagine that screen readers would announce multiple focus jumps (not sure tho), that may require additional tweaks.

Changed the implementation to focus after the hidden event dispatched. If a user manually moves the focus in the event outside of the dropdown, it shouldn't interfere with it anyway. Added a test to the spec to confirm that. As well as another test to make sure no error is thrown if the menu is disposed in the hidden event.

Type of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Live previews

Related issues

Closes #35793
Closes #41588 (tbf this can be closed already, just so we don't forget about it)

@alpadev alpadev requested a review from a team as a code owner August 31, 2025 08:49
@alpadev alpadev changed the title Enforce focus on the dropdown trigger Enforce focus on dropdown trigger Aug 31, 2025
@alpadev
test: add additional tests to the dropdown spec
200d26e 
Allow users to move the focus in the hidden event, without interfering.
Make sure if dispose is called in the hidden event, no error is thrown.
@alpadev
refactor: Focus dropdown after hidden event
d9c14fb 
Allow users to move the focus manually in the hidden event without interfering.
XhmikosR
XhmikosR reviewed Sep 1, 2025
View reviewed changes
@alpadev
refactor: don't use optional chaining
a9b3acc 
Because of transpiling using optional chaining is suboptimal
@alpadev
Merge branch 'main' into fix-dropdown-focus
903707d
@github-project-automation github-project-automation bot moved this to Needs review in v5.4.0 Sep 2, 2025
@tulsaskaterchic
Copy link

tulsaskaterchic commented Nov 13, 2025 •
edited
Loading

Will this also fix ensuring focus doesn't return if selecting a different dropdown entirely? We've had to locally comment out the focus line at the end of the hide function because we have menus with sub-menus and it caused the following unwanted behavior.

Example: a menu with two dropdown sub-menus. Click on Submenu 2 and its contents appear.
image

Then click on Submenu 1. Submenu 2 closes as expected and Submenu 1 opens as expected but then focus in returned back to Submenu 2 which is no longer the active menu item.
image

Menu setup for the demo:

<li class="nav-item dropdown">
     <a class="nav-link dropdown-toggle" href="#" role="button" data-bs-toggle="dropdown" data-bs-auto-close="outside" aria-expanded="false" id="menuDemo">Demo</a>
     <ul class="dropdown-menu">
         <li class="dropend">
             <a class="dropdown-item dropdown-toggle " data-bs-toggle="dropdown" data-bs-auto-close="outside" aria-expanded="false" href="#" id="subMenuDemo1">Submenu 1</a>
             <ul class="dropdown-menu dropdown-menu-end">
                 <li>Menu Item 1A</li>
                 <li>Menu Item 1B</li>
             </ul>
         </li>         
         <li class="dropend">
             <a class="dropdown-item dropdown-toggle " data-bs-toggle="dropdown" data-bs-auto-close="outside" aria-expanded="false" href="#" id="subMenuDemo2">Submenu 2</a>
             <ul class="dropdown-menu dropdown-menu-end">
                 <li>Menu Item 2A</li>
                 <li>Menu Item 2B</li>
                 <li>Menu Item 2C</li>
             </ul>
         </li>
     </ul>
 </li>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@XhmikosR XhmikosR XhmikosR left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

accessibility js

Projects

v5.4.0
Status: Needs review

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

dropdown.js: _completeHide results in uncaught TypeError A11y: Closing a dropdown should return focus to the dropdown control

4 participants

@alpadev @tulsaskaterchic @XhmikosR @julien-deramond